-
-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add @metamask/wallet
package [WIP]
#4105
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
The `@metamask/wallet` package is a library that is intended for building MetaMask wallets. It will provide an API for all functionality supported across all MetaMask wallts, minus the UI. The first release of this package will not support all MetaMask wallet functionality. The initial version will support keyrings, accounts, approvals, preferences, network access, fiat currency rates, gas estimates, and transactions. We will add additional features one-by-one until this library includes all common functionality. This MVP has not yet been implemented. I've written a type declaration and some non-functional unit tests to demonstrate roughly what the API would look like, to get early feedback. This will be written up as an architecture decision record before proceeding with the implementation.
96f5f5e
to
17a17e8
Compare
|
||
// Additional jotnotes: | ||
// | ||
// Step0: Rename controller messenger, write ADR about services and selectors, refactor guts of ComposableController into compose function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor guts of ComposableController into compose function
I haven't yet demonstrated in this PR how we'd combine the state of this wallet library with additional controllers. But my idea was to extract the internals of ComposableController
into a function, which could be used both internally in the wallet library, and externally by the client to compose additional controller states.
* MetaMask wallet state. | ||
*/ | ||
export type MetamaskState< | ||
ControllerPermissionSpecification extends PermissionSpecificationConstraint & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making the permissions extensible ends up being quite verbose. Surely there's a more concise way of doing this, but I haven't experimented with it much yet.
subjectType: SubjectType; | ||
}): JsonRpcEngine; | ||
|
||
// TODO: Add start/resume, pause, stop methods to control all polling/services |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Establishing a standard approach for polling might be necessary for this.
The services part is easy, we can add an enableServices
/disableServices
method to handle that. But I am not sure on polling. Currently we don't even use actions for it, we use methods.
Probably we'll want to standardize some add/remove polling token actions, then add a method to pause/unpause all polling globally.
> { | ||
#controllerMessenger: ControllerMessenger<AllWalletActions, AllWalletEvents>; | ||
|
||
#controllers: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the initial set of controllers that I'll be proposing for the MVP.
I wanted enough functionality to demonstrate that the JSON-RPC pipeline works and is useful, so the NetworkController was a must. I brought in the TransactionController as well because I wanted at least one intercepted method, and it seemed easier to bring that in than the SignatureController (because it's already on BaseControllerV2, but the message managers are not).
And the GasFeeController was brought in because it's a peer dependency of the TransactionController. Same for the ApprovalController. Though in both cases there are other reasons to include them as well: the GasFeeController is a nice example of a polling controller, and the ApprovalController establishes how we handle user confirmations.
CurrencyRateController was the one asset controller that seemed easy to bring on its own, and it lets us demonstrate more complex wallet API interactions down the line (e.g. when we go to write an integration test to approximate what the UI might want to render a confirmation).
AccountsController and PreferencesController were brought in for the identity state, which is still in both places at the moment. Even after it's removed from preferences, wallet preferences is nice to have on hand. Later I'll be suggesting that we audit the preferences to ensure they're all relevant to the wallet library, moving anything else to a MobilePreferencesController.
Lastly, KeyringController and PermissionController need to be included, they're fundamental to what the wallet is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was tempted to include snaps even in this early draft, but that seemed pretty complicated to integrate. And it's not completely landed on mobile yet.
Plus, snaps provides a nice demonstration of why we need extensible permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've looked this over and it matches more or less what I was picturing in my mind. Assuming we are able to keep this simple API, then this looks really great. Anxious to see the tests passing, but they are simple to understand at least as well. Looks good!
(I know this is a draft — the approval is just meant to cover the fact that the POC nature of this draft looks good. If it gets fleshed out, then I'll re-review with those new changes in mind.)
Superseded by #4451 |
Explanation
The
@metamask/wallet
package is a library that is intended for building MetaMask wallets. It will provide an API for all functionality supported across all MetaMask wallts, minus the UI.The first release of this package will not support all MetaMask wallet functionality. The initial version will support keyrings, accounts, approvals, preferences, network access, fiat currency rates, gas estimates, and transactions. We will add additional features one-by-one until this library includes all common functionality.
This MVP has not yet been implemented. I've written a type declaration and some non-functional unit tests to demonstrate roughly what the API would look like, to get early feedback. This will be written up as an architecture decision record before proceeding with the implementation.
References
Relates to https://github.com/MetaMask/MetaMask-planning/issues/1861
Changelog
@metamask/wallet
Added
Checklist